test/module-formats.test.js (1)

49-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated key-parity check across two tests.

The key-parity loop at Lines 61–63 (for (const key of Object.keys(esm.default)) { assert.ok(Object.hasOwn(cjs, key), ...) }) is repeated verbatim at Lines 73–75 in the next test. Consider consolidating into a shared helper or merging the assertions into a single test to avoid drift.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/module-formats.test.js` around lines 49 - 76, Consolidate the duplicated
ESM/CJS key-parity loop currently present in both tests. Either merge the
assertions into one test or extract a shared helper and reuse it from the tests,
while preserving the existing missing-key assertion message and API checks.

node-ical.d.ts (1)

112-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider more precise types for internal properties currently typed unknown.

objectHandlers, handleObject, and parseLines are typed as unknown, which gives consumers no type safety if they access these properties. If this is intentional to discourage direct use, consider adding a brief comment explaining that. Otherwise, typing them (e.g., as Record<string, (...args: unknown[]) => unknown> and (...args: unknown[]) => unknown) would improve the developer experience for advanced consumers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@node-ical.d.ts` around lines 112 - 125, Improve the declarations for
objectHandlers, handleObject, and parseLines in the default export object:
either replace unknown with accurate callable/record types for these internal
properties, or add a brief comment documenting that unknown is intentional to
discourage direct consumer use. Preserve the existing exported API shape and
type the properties consistently.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

test/module-formats.test.js (1)

49-76: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Duplicated key-parity check across two tests.

The key-parity loop at Lines 61–63 (for (const key of Object.keys(esm.default)) { assert.ok(Object.hasOwn(cjs, key), ...) }) is repeated verbatim at Lines 73–75 in the next test. Consider consolidating into a shared helper or merging the assertions into a single test to avoid drift.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/module-formats.test.js` around lines 49 - 76, Consolidate the duplicated
ESM/CJS key-parity loop currently present in both tests. Either merge the
assertions into one test or extract a shared helper and reuse it from the tests,
while preserving the existing missing-key assertion message and API checks.

node-ical.d.ts (1)

112-125: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Consider more precise types for internal properties currently typed unknown.

objectHandlers, handleObject, and parseLines are typed as unknown, which gives consumers no type safety if they access these properties. If this is intentional to discourage direct use, consider adding a brief comment explaining that. Otherwise, typing them (e.g., as Record<string, (...args: unknown[]) => unknown> and (...args: unknown[]) => unknown) would improve the developer experience for advanced consumers.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@node-ical.d.ts` around lines 112 - 125, Improve the declarations for
objectHandlers, handleObject, and parseLines in the default export object:
either replace unknown with accurate callable/record types for these internal
properties, or add a brief comment documenting that unknown is intentional to
discourage direct consumer use. Preserve the existing exported API shape and
type the properties consistently.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

build/update-windows-zones.js (1)

18-45: 🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify AbortSignal.timeout() and fetch() availability in Node 20+
node -e "
const hasFetch = typeof fetch === 'function';
const hasAbortTimeout = typeof AbortSignal.timeout === 'function';
console.log('fetch:', hasFetch);
console.log('AbortSignal.timeout:', hasAbortTimeout);
if (!hasFetch || !hasAbortTimeout) process.exit(1);
"

Repository: jens-maus/node-ical

Length of output: 195

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

# Inspect the file structure and relevant symbols
ast-grep outline build/update-windows-zones.js --view expanded || true

# Read the file with line numbers
wc -l build/update-windows-zones.js
cat -n build/update-windows-zones.js | sed -n '1,220p'

# Check whether https is used elsewhere in the file
rg -n "\bhttps\b|fetchText|AbortSignal\.timeout|globalThis\.fetch|fetch\(" build/update-windows-zones.js

Repository: jens-maus/node-ical

Length of output: 7600

---

Add a timeout to fetchText to avoid CI hangs.
https.get() here can wait indefinitely if the CLDR endpoint stalls. Since this script runs on Node 20+, switching to fetch() with AbortSignal.timeout() would also remove the manual redirect handling.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@build/update-windows-zones.js` around lines 18 - 45, Update fetchText to
enforce a request timeout so stalled CLDR requests reject instead of hanging CI.
Prefer replacing the manual https.get redirect flow with Node 20’s fetch and
AbortSignal.timeout(), while preserving non-success HTTP errors and returning
the response text for successful requests.

ical.js (1)

463-475: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Bind named exports that depend on this.

Directly importing handleObject, parseLines, or parseICS calls them with this === undefined; each accesses another method or property through this and will fail. Bind these exports to ical. The equivalent exports in node-ical.js need the same treatment.

Proposed fix

 const {
   objectHandlers,
-  handleObject,
-  parseLines,
-  parseICS,
 } = ical;
+const handleObject = ical.handleObject.bind(ical);
+const parseLines = ical.parseLines.bind(ical);
+const parseICS = ical.parseICS.bind(ical);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

const {
  objectHandlers,
} = ical;
const handleObject = ical.handleObject.bind(ical);
const parseLines = ical.parseLines.bind(ical);
const parseICS = ical.parseICS.bind(ical);

export {
  objectHandlers,
  handleObject,
  parseLines,
  parseICS,
};

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@ical.js` around lines 463 - 475, Update the named exports for handleObject,
parseLines, and parseICS in ical.js to bind each function to the ical object so
calls retain the required this context; apply the same binding treatment to the
equivalent exports in node-ical.js, while leaving objectHandlers unchanged.

lib/expand-recurring-event.js (2)

94-120: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Use calendar-day spans for full-day event ends.

Adding a fixed millisecond duration to local midnight produces 23:00 or 01:00 ends around DST transitions. Track full-day duration as a number of calendar dates and advance with local calendar arithmetic.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/expand-recurring-event.js` around lines 94 - 120, Update
getEventDurationMs and calculateEndTime to represent full-day durations as
calendar-day counts rather than fixed milliseconds. For full-day events, compute
the span between local calendar dates and advance the start using local date
arithmetic so DST transitions preserve the correct end date; keep timed-event
millisecond duration behavior unchanged.

---

345-358: 🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Include overrides moved into the requested range.

Only RRULE dates inside searchFrom..searchTo are inspected. An override keyed by an earlier RECURRENCE-ID but moved into this range is therefore omitted. Merge unique event.recurrences whose effective starts overlap the range, while deduplicating overrides already reached through RRULE expansion.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/expand-recurring-event.js` around lines 345 - 358, Update the
recurring-instance expansion around buildRecurringInstance so it also inspects
event.recurrences overrides whose effective start falls within the requested
range, even when their RECURRENCE-ID is outside searchFrom..searchTo. Merge
these override-derived instances with the RRULE results, deduplicate instances
already produced through RRULE expansion, then retain the existing range
filtering and start-time sorting.

lib/ical-parser-utils.js (3)

41-46: 🔒 Security & Privacy | 🔴 Critical | ⚡ Quick win

Store UID keys without invoking object prototype setters.

A UID of __proto__ resolves to Object.prototype; the merge or recurrence path then writes parsed event fields onto the global prototype. Use Object.hasOwn, copy only own fields, and define UID properties with Object.defineProperty or use null-prototype records.

Proposed approach

+function setRecordField(target, key, value) {
+  Object.defineProperty(target, key, {
+    value,
+    enumerable: true,
+    configurable: true,
+    writable: true,
+  });
+}
+
 function copyRecordFields(target, source) {
-  for (const key in source) {
-    if (key !== null) {
-      target[key] = source[key];
-    }
+  for (const key of Object.keys(source)) {
+    setRecordField(target, key, source[key]);
   }
 }
...
-  if (parentEntry[uid] === undefined) {
-    parentEntry[uid] = sourceEntry;
+  if (!Object.hasOwn(parentEntry, uid)) {
+    setRecordField(parentEntry, uid, sourceEntry);

Also applies to: 99-120

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/ical-parser-utils.js` around lines 41 - 46, Update copyRecordFields to
iterate and copy only source own properties using Object.hasOwn, and avoid
assignment-based writes for the __proto__ UID by defining UID-keyed properties
with Object.defineProperty or using null-prototype records throughout the
related merge/recurrence path. Apply the same protection to the code around the
UID handling at the referenced later section, preserving normal field-copy
behavior for other keys.

---

185-194: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Advance DATE values by a calendar day, not 24 elapsed hours.

utcAdd(..., 1, 'days') adds exactly 24 hours. Across DST transitions, the implicit end can become 23:00 or 01:00 instead of local midnight on the next date.

Proposed fix

   if (entry.duration === undefined) {
-    entry.end = entry.datetype === 'date-time'
-      ? cloneDateWithMeta(entry.start)
-      : cloneDateWithMeta(entry.start, utcAdd(entry.start, 1, 'days'));
+    entry.end = cloneDateWithMeta(entry.start);
+    if (entry.datetype !== 'date-time') {
+      entry.end.setDate(entry.end.getDate() + 1);
+    }
     return entry;
   }

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

function applyImplicitEndDate(entry, utcAdd) {
  if (entry.end) {
    return entry;
  }

  if (entry.duration === undefined) {
    entry.end = cloneDateWithMeta(entry.start);
    if (entry.datetype !== 'date-time') {
      entry.end.setDate(entry.end.getDate() + 1);
    }
    return entry;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/ical-parser-utils.js` around lines 185 - 194, Update applyImplicitEndDate
for DATE entries to advance entry.start by one calendar day while preserving
local midnight, rather than using utcAdd(..., 1, 'days'). Keep the existing
date-time cloning behavior and return flow unchanged.

---

721-733: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win

Invoke the FREEBUSY storage handler.

storeParameter(value, parameters, fb) only creates and discards a handler, so the raw FREEBUSY value and its parameters are never stored.

Proposed fix

-      storeParameter(value, parameters, fb);
+      storeParameter(name)(value, parameters, fb);

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

  const freebusyParameter = function (name) {
    return function (value, parameters, curr) {
      const fb = addFBType({}, parameters);
      curr[name] ||= [];
      curr[name].push(fb);

      storeParameter(name)(value, parameters, fb);

      const parts = value.split('/');

      for (const [index, partName] of ['start', 'end'].entries()) {
        dateParameter(partName)(parts[index], parameters, fb);
      }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/ical-parser-utils.js` around lines 721 - 733, Update freebusyParameter so
it invokes the FREEBUSY storage handler returned or registered by
storeParameter, rather than merely calling storeParameter and discarding its
result. Ensure the raw value and parameters are persisted on fb before
dateParameter processes the start and end parts.

package.json (1)

4-24: 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Check the engines.node field and verify compatibility with import attributes
cat package.json | jq '.engines'

Repository: jens-maus/node-ical

Length of output: 178

---

🏁 Script executed:

#!/bin/bash
set -euo pipefail

printf '\n## package.json engines and module fields\n'
cat -n package.json | sed -n '1,80p'

printf '\n## referenced source snippets\n'
for f in lib/tz-utils.js lib/temporal.js; do
  echo "\n### $f"
  cat -n "$f" | sed -n '1,220p'
done

printf '\n## shipped entry files\n'
git ls-files 'node-ical.*' 'lib/*.js' 'windowsZones.json'

Repository: jens-maus/node-ical

Length of output: 11636

---

Bump engines.node to >=20.10.0. lib/tz-utils.js uses JSON import attributes, so >=20 still admits Node 20.0–20.9, which will fail to load the ESM entry.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@package.json` around lines 4 - 24, Update the package.json engines.node
constraint to require Node.js >=20.10.0, ensuring the ESM entry using JSON
import attributes is not exposed to unsupported Node 20.0–20.9 versions.

Source: Learnings

test/examples-snapshot.test.js (1)

54-54: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Remove unnecessary v flag from regular expression.

The v (Unicode sets) flag is not required for this simple extension-matching regular expression. Additionally, since the v flag was introduced in Node.js 20, using it may cause a SyntaxError if this test is executed in older Node.js environments that might still be supported.

Consider removing the v flag for improved compatibility and simplicity.

♻️ Proposed fix

-      const snapshotFile = path.join(snapshotDir, script.replace(/\.(?:c|m)?js$/v, '.txt'));
+      const snapshotFile = path.join(snapshotDir, script.replace(/\.(?:c|m)?js$/, '.txt'));

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@test/examples-snapshot.test.js` at line 54, Update the regular expression in
the snapshotFile path construction to remove the unnecessary v flag, while
preserving its existing extension-matching behavior and compatibility with older
Node.js versions.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

node-ical.d.ts (1)

112-128: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Explicitly use declare for ambient const declarations.

If this block is at the top level of the .d.ts module, omitting declare or an initializer could cause a TypeScript TS1155 compilation error for consumers. Even if it is nested within an ambient namespace or module where declare is implicit, adding it explicitly improves readability and aligns with standard type definition practices.

✨ Proposed refactor

-  const _default: {
+  declare const _default: {
     fromURL: typeof fromURL;
     parseFile: typeof parseFile;
     parseICS: typeof parseICS;
     sync: typeof sync;
     async: typeof async;
     expandRecurringEvent: typeof expandRecurringEvent;
     /** Internal compatibility hooks; intentionally left loose to avoid encouraging direct use. */
     objectHandlers: unknown;
     /** Internal compatibility hooks; intentionally left loose to avoid encouraging direct use. */
     handleObject: unknown;
     /** Internal compatibility hooks; intentionally left loose to avoid encouraging direct use. */
     parseLines: unknown;
   };

   export default _default;

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@node-ical.d.ts` around lines 112 - 128, Update the top-level ambient constant
declaration for _default to explicitly use declare, while preserving its
existing property types and default export.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

lib/ical-parser-utils.js (1)

133-141: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win

Fully replace override-first entries when the base series arrives.

This branch still overlays base fields onto the override, leaving override-only properties on the resulting base event. Clear removed fields before copying; recurrences and method are already preserved.

Proposed fix

   if (existingIsRecurrence && incomingIsBaseSeries) {
+    clearFieldsRemovedInRevision(existingEntry, incomingEntry);
     copyRecordFields(existingEntry, incomingEntry);
     return;
   }

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@lib/ical-parser-utils.js` around lines 133 - 141, Update
applyUidSequenceMerge’s existingIsRecurrence/incomingIsBaseSeries branch to
clear the existing override record’s removed fields before copying base-series
fields, while preserving recurrences and method as currently handled. Ensure the
resulting entry is fully replaced by the incoming base series rather than
retaining override-only properties.

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes